Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

build: Fix libpq for Docker multi-arch builds and Bookworm upgrade #7921

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

build: Fix libpq for Docker multi-arch builds and Bookworm upgrade #7921

wants to merge 4 commits into from

Conversation

s373nZ
Copy link
Contributor

@s373nZ s373nZ commented Dec 9, 2024 •
edited
Loading

This PR adds upon the excellent initial work done by @arowser in #7827 on cross-compiling libpq through the multi-architecture Docker release builds. Exhaustive testing exposed some local Docker build errors which have been addressed in additional commits.

Upon running into some local QEMU segmentation faults while cross compiling, we noticed that qemu-user-static was on a rather old version of 5.2 under Debian Bullseye. Therefore, we also update the Docker release base images to Bookworm, pushing QEMU to 7.2. Bullseye is "old stable" and Bookworm is now "stable", so there is a case to be made for upgrading.

Additionally, Python is upgraded to 3.11 as a corollary. Currently setting pip flags to break system packages, mainly in the interest of time, but this could be addressed to use virtual environments if required.

  • Fixed wget.
  • Added Postgres build dependencies libicu-dev, bison and flex.
  • Removed the existing libpq Debian package installs.
  • Defined POSTGRES_CONFIG declaration for each cross compile target.
  • Defined PG_CONFIG for all targets.
  • Used pg_config to run ldconfig, making compiled libraries available to the system.
  • Copied the libpq libraries from the build stage to the final stage using a bind mount RUN command.
  • Upgraded Debian base image to Bookworm.
  • Upgraded Python to 3.11.
  • Clean up Dockerfile formatting and standardize the use of ENV directives when the values are not interdependent.

The full multi-arch build has been tested locally by building on amd64. Still need to figure out a way to get the arm target image onto a Mac to test libpq from inside a container.

Fixes #7649
Relates to and duplicates #7827

Respectfully, these changes have also been submitted in a PR against @arowser's branch here. As there is a bounty posted for #7649, I yield to the CLN core teams determination on who should win the payout or on whether / how to divide it. 🙏

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@s373nZ s373nZ force-pushed the 7827-libpq-multiarch--fixes branch from da58237 to 39f5b1a Compare December 11, 2024 13:11
@s373nZ
Copy link
Contributor Author

s373nZ commented Dec 11, 2024

Sooo... after some more rigorous testing, the first iteration actually broke libpq availability for all targets. Although libpq was installed correctly, the libraries needed to also be added to the linker for configurator to find them to signal yes for Postgres. Also needed to be done in the final build stage. The recent changes:

  • Take care to set PG_CONFIG for the compilation correctly for all targets as they drop build artifacts in different places.
  • Uses pg_config to run ldconfig on the compiled libpq libraries during the build phase.
  • Copies the libpq libraries to another location and uses a bind mount to copy them from the builder image into the final image at /usr/local/pgsql/lib and then runs ldconfig on that directory to make them available.
  • Cleans up Dockerfile formatting and standardizes the use of multi-line ENV directives when the values don't depend on each other.

This has been tested by running a cross-platform build on an amd64 machine and then running the target images directly and executing the following:

root@25584da367b1:/# lightningd --wallet=postgres://test:test@localhost:5432/lightning
2024-12-11T13:22:41.967Z INFO    lightningd: v24.11-3-g7dbe807
2024-12-11T13:22:44.641Z UNUSUAL plugin-bookkeeper: topic 'utxo_deposit' is not a known notification topic
2024-12-11T13:22:44.641Z UNUSUAL plugin-bookkeeper: topic 'utxo_spend' is not a known notification topic
2024-12-11T13:22:44.826Z **BROKEN** lightningd: Error calling DB setup: Could not connect to postgres://test:test@localhost:5432/lightning: connection to server at "localhost" (::1), port 5432 failed: Connection refused??Is the server running on that host and accepting TCP/IP connections??connection to server at "localhost" (127.0.0.1), port 5432 failed: Connection refused??Is the server running on that host and accepting TCP/IP connections??
Error calling DB setup: Could not connect to postgres://test:test@localhost:5432/lightning: connection to server at "localhost" (::1), port 5432 failed: Connection refused
        Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 5432 failed: Connection refused
        Is the server running on that host and accepting TCP/IP connections?

This error indicates libpq is available, without having to configure the whole node. Tested the arm64 and armv7 from the amd64 machine to the same success. Still haven't ported the actual cross platform images to a Mac to test natively.

The learning from testing this has prompted considerations on trying to add static linking support for libpq in the Makefile, which might be tried in a separate prototype.

@s373nZ
Copy link
Contributor Author

s373nZ commented Dec 11, 2024

Managed to test the cross-compiled images on an M2 Mac and Raspberry Pi 4, both arm64, with successful results per the above methodology. 🎉

Don't have an armv7 device to test with Docker.

@rustyrussell rustyrussell added this to the v25.02 milestone Dec 15, 2024
@s373nZ s373nZ force-pushed the 7827-libpq-multiarch--fixes branch from 39f5b1a to abd6276 Compare December 18, 2024 15:56
@s373nZ
Copy link
Contributor Author

s373nZ commented Dec 18, 2024

Rebased against master.

ShahanaFarooqui
ShahanaFarooqui requested changes Dec 21, 2024
View reviewed changes
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@s373nZ
Copy link
Contributor Author

s373nZ commented Dec 21, 2024

Thanks for the review! So the only TODO is to look into installing the pip packages alternatively, w/o PIP_BREAK_SYSTEM_PACKAGES=1, right? If so, I'll start looking at that and resolve the other comments.

@ShahanaFarooqui
Copy link
Collaborator

If so, I'll start looking at that and resolve the other comments.

Yes, that should be good enough.

arowser and others added 4 commits December 22, 2024 17:51
@arowser @s373nZ
add libpq for multi-arch build configuration
232e6ef
@s373nZ
build: Fixes for the Docker building libpq lib.
acd3a39 
- Add Postgres dependencies: bison, flex and libiu-dev.
- Fix missing `&&` in chained wget commands.
- Add `POSTGRES_CONFIG` and `PG_CONFIG` for all architectures.
- Remove existing `libpq` Ubuntu packages.
- Copy libpq libraries from builder directly to final image.

Changelog-Fixed: Fixes Postgres driver availability for arm64 and arm32 Docker images.
@s373nZ
build: Upgrade Docker base images to Debian Bookworm.
b3a1f10 
Undertaken to upgrade QEMU to 7.2. Also upgrades Python to 3.11
implicitly and migrates Python dependency management to virtual environments.

Changelog-Changed: Released Docker images are now based on Debian Bookworm
@s373nZ
chore(docker): Clean up formatting and standardize Dockerfile.
3dabe66 
- Align indentation.
- Use multi-line `ENV` where values don't depend on each other.

Changelog-None
@s373nZ s373nZ force-pushed the 7827-libpq-multiarch--fixes branch from abd6276 to 3dabe66 Compare December 22, 2024 16:52
@s373nZ
Copy link
Contributor Author

s373nZ commented Dec 22, 2024

  • Manage Python dependencies in virtual environments.
  • Put libpq library location definition in /etc/ld.so.conf.d/libpq.conf in the final image to be more "Debian-like".
  • Rebased against master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShahanaFarooqui ShahanaFarooqui ShahanaFarooqui requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v25.02
Development

Successfully merging this pull request may close these issues.

Postgres driver not found in ARM64 and ARM32 Docker images
4 participants
@s373nZ @ShahanaFarooqui @rustyrussell @arowser